frontend/app/aipanel/aitooluse.tsx (1)

205-212: Apply session approvals before sending final approve action.

At Line 211 and Line 291, handleApprove() runs immediately after firing session-approval RPCs. If those calls are delayed/fail, subsequent reads may still prompt unexpectedly despite the user choosing session allow.

💡 Proposed fix

-    const handleAllowSession = () => {
+    const handleAllowSession = async () => {
         const dirs = extractDirsFromParts(parts);
         const model = WaveAIModel.getInstance();
-        for (const dir of dirs) {
-            model.sessionReadApprove(dir);
-        }
+        await Promise.allSettled(dirs.map((dir) => model.sessionReadApprove(dir)));
         handleApprove();
     };
@@
-    const handleAllowSession = () => {
+    const handleAllowSession = async () => {
         const dir = extractDirFromToolDesc(toolData.tooldesc);
         if (dir) {
-            WaveAIModel.getInstance().sessionReadApprove(dir);
+            await WaveAIModel.getInstance().sessionReadApprove(dir);
         }
         handleApprove();
     };

Also applies to: 286-292

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@frontend/app/aipanel/aitooluse.tsx` around lines 205 - 212, The handler
handleAllowSession calls WaveAIModel.getInstance().sessionReadApprove(dir) in a
loop then immediately calls handleApprove(), which can race with async RPCs;
make handleAllowSession async, collect the promises returned by
sessionReadApprove (via extractDirsFromParts(parts)), await
Promise.all(promises) and only then call handleApprove(); apply the same change
to the other similar handler that performs sessionReadApprove before
handleApprove so approvals complete before sending the final approve action.

pkg/aiusechat/sessionapproval.go (1)

62-99: ⚠️ Potential issue | 🟠 Major

Normalize both sides before sensitive/prefix checks (case + canonical roots).

Line 67/Line 83 and Line 115 compare path strings directly, but only the candidate path is canonicalized. Sensitive roots and approval roots are not normalized to the same comparison form (notably case on Windows, and alias/symlink variants of protected roots), so sensitive-path detection and approval matching can be bypassed or behave inconsistently.

Suggested fix

 import (
 	"os"
 	"path/filepath"
+	"runtime"
 	"strings"
 	"sync"
@@
 func canonicalizePath(rawPath string) string {
@@
 	return resolved
 }
+
+func normalizePathForCompare(rawPath string) string {
+	p := filepath.Clean(canonicalizePath(rawPath))
+	if runtime.GOOS == "windows" {
+		p = strings.ToLower(p)
+	}
+	return p
+}
@@
 func AddSessionReadApproval(dirPath string) {
-	canonical := canonicalizePath(dirPath)
+	canonical := normalizePathForCompare(dirPath)
@@
 func isSensitivePath(expandedPath string) bool {
-	homeDir := os.Getenv("HOME")
-	if homeDir == "" {
-		homeDir = os.Getenv("USERPROFILE")
-	}
-	cleanPath := filepath.Clean(expandedPath)
+	homeDir, _ := os.UserHomeDir()
+	cleanPath := normalizePathForCompare(expandedPath)
@@
 	for _, dir := range sensitiveDirs {
-		dirWithSep := dir + string(filepath.Separator)
-		if cleanPath == dir || strings.HasPrefix(cleanPath, dirWithSep) {
+		normalizedDir := normalizePathForCompare(dir)
+		dirWithSep := normalizedDir + string(filepath.Separator)
+		if cleanPath == normalizedDir || strings.HasPrefix(cleanPath, dirWithSep) {
 			return true
 		}
 	}
@@
 	if localAppData := os.Getenv("LOCALAPPDATA"); localAppData != "" {
-		credPath := filepath.Join(localAppData, "Microsoft", "Credentials")
+		credPath := normalizePathForCompare(filepath.Join(localAppData, "Microsoft", "Credentials"))
 		if cleanPath == credPath || strings.HasPrefix(cleanPath, credPath+string(filepath.Separator)) {
 			return true
 		}
 	}
 	if appData := os.Getenv("APPDATA"); appData != "" {
-		credPath := filepath.Join(appData, "Microsoft", "Credentials")
+		credPath := normalizePathForCompare(filepath.Join(appData, "Microsoft", "Credentials"))
 		if cleanPath == credPath || strings.HasPrefix(cleanPath, credPath+string(filepath.Separator)) {
 			return true
 		}
 	}
@@
 func IsSessionReadApproved(filePath string) bool {
-	canonical := canonicalizePath(filePath)
+	canonical := normalizePathForCompare(filePath)

#!/bin/bash
set -euo pipefail

echo "Inspect current comparison normalization in session approval logic:"
sed -n '27,125p' pkg/aiusechat/sessionapproval.go

echo
echo "Check whether case normalization is currently present:"
rg -n 'ToLower|EqualFold|runtime\.GOOS' pkg/aiusechat/sessionapproval.go

echo
echo "Check whether sensitive roots are canonicalized before comparison:"
rg -n 'sensitiveDirs|isSensitivePath|canonicalizePath\(|normalizePathForCompare' pkg/aiusechat/sessionapproval.go

Also applies to: 107-116

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@pkg/aiusechat/sessionapproval.go` around lines 62 - 99, The isSensitivePath
logic compares only the candidate path after filepath.Clean, which allows
mismatches on case-insensitive OSes and symlink/alias variants; update
isSensitivePath (and the related approval-root checks around lines referenced)
to canonicalize and normalize both sides before comparing: resolve abs +
EvalSymlinks (or fall back to Clean), normalize separators, and on Windows use
case-insensitive comparison (strings.EqualFold) or strings.ToLower for both the
cleaned expandedPath and each sensitiveDir/approvalRoot; ensure you canonicalize
every entry in sensitiveDirs (and the LOCALAPPDATA/APPDATA derived credPath)
prior to the equality/HasPrefix checks and use the same dirWithSep normalization
for prefix checks.

pkg/aiusechat/tools_readfile.go (1)

405-417: ⚠️ Potential issue | 🔴 Critical

Still unresolved: session auto-approve reaches unblocked ~/.ssh and ~/.aws configs.

At Line 414, any file under a session-approved parent becomes auto-approved, but isBlockedFile() still only blocks key-like files and a few exact paths. ~/.ssh/config, ~/.aws/config, and similar config files still pass ToolVerifyInput, so approving those directories for the session exposes sensitive material.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@pkg/aiusechat/tools_readfile.go` around lines 405 - 417, The session
auto-approve logic in ToolApproval can incorrectly approve sensitive config
files via an approved parent directory; update the closure in tools_readfile.go
so that after expanding the path (ExpandHomeDir(parsed.Filename)) you first
check isBlockedFile(expandedPath) (or a new helper that checks for blocked
ancestors) and return uctypes.ApprovalNeedsApproval if blocked, and only then
call IsSessionReadApproved(expandedPath) to return ApprovalAutoApproved; ensure
you reference parseReadTextFileInput, wavebase.ExpandHomeDir,
IsSessionReadApproved and isBlockedFile (or implement a blocked-ancestor check)
so ToolVerifyInput cannot be bypassed by approving a parent directory.

frontend/app/monaco/monaco-react.tsx-141-144 (1)

141-144: ⚠️ Potential issue | 🟡 Minor

Avoid extension parsing from the full path

Line 141-Line 142 parse extensions from the entire path, so a dot in a directory name can be misread as a file extension and produce broken diff URIs.

💡 Proposed fix

-        const ext = path.includes(".") ? path.substring(path.lastIndexOf(".")) : "";
-        const basePath = path.includes(".") ? encodeURIComponent(path.substring(0, path.lastIndexOf("."))) : encodeURIComponent(path);
+        const lastSlash = path.lastIndexOf("/");
+        const dir = lastSlash >= 0 ? path.slice(0, lastSlash + 1) : "";
+        const fileName = lastSlash >= 0 ? path.slice(lastSlash + 1) : path;
+        const lastDot = fileName.lastIndexOf(".");
+        const hasExt = lastDot > 0;
+        const ext = hasExt ? fileName.slice(lastDot) : "";
+        const basePath = encodeURIComponent(hasExt ? `${dir}${fileName.slice(0, lastDot)}` : path);

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@frontend/app/monaco/monaco-react.tsx` around lines 141 - 144, The code
currently derives ext and basePath by looking for a dot anywhere in the full
path, which mis-detects dots in directory names; update the logic that computes
ext and basePath used for origUri and modUri to first isolate the filename (use
path.basename on the incoming path) and then compute the extension and
name-from-filename (i.e., find the last dot on the filename only). Change the
variables ext and basePath (used when building origUri and modUri) so they are
based on the filename-derived values (encoded as before) to avoid treating
directory dots as file extensions.

frontend/app/view/codeeditor/diffviewer.tsx-35-42 (1)

35-42: ⚠️ Potential issue | 🟡 Minor

Make compound extension detection case-insensitive

Line 41 only matches lowercase "blade.php", so files like View.BLADE.PHP won’t get the intended html language.

💡 Proposed fix

 function getLanguageFromFileName(fileName: string): string | undefined {
-    const ext = fileName.split(".").pop()?.toLowerCase();
-    // Handle compound extensions like .blade.php
-    const parts = fileName.split(".");
-    if (parts.length >= 3) {
-        const compound = parts.slice(-2).join(".");
-        if (compound === "blade.php") return "html";
-    }
+    const normalized = fileName.toLowerCase();
+    if (normalized.endsWith(".blade.php")) return "html";
+    const ext = normalized.split(".").pop();
     return ext ? extToLanguage[ext] : undefined;
 }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@frontend/app/view/codeeditor/diffviewer.tsx` around lines 35 - 42, The
compound-extension check in getLanguageFromFileName only matches "blade.php" in
lowercase so files like View.BLADE.PHP are missed; update the comparison to be
case-insensitive by normalizing the compound variable (e.g., call toLowerCase()
on parts.slice(-2).join(".")) before comparing and ensure any other
compound-checks use the same normalization (refer to function
getLanguageFromFileName and the compound variable).

pkg/aiusechat/projectctx/projectctx.go-137-140 (1)

137-140: ⚠️ Potential issue | 🟡 Minor

Truncation can break the closing tag.

At Lines 137-140, result[:maxLen] is taken after </project_instructions> has already been appended. If the slice lands inside that closing tag, the output contains a partial terminator plus another full terminator. Truncate the body first, then append the closing tag once.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@pkg/aiusechat/projectctx/projectctx.go` around lines 137 - 140, The
truncation currently slices `result` after the closing tag
`</project_instructions>` has been appended, which can split that tag; change
the logic in the function that builds `result` so you truncate the main body
before appending the closing tag: compute/truncate the body to `maxLen` (use the
same const maxLen = 8000) and then append `"\n...
[truncated]\n</project_instructions>"` (or the full closing tag) once; update
any references to `result` accordingly so the final `result` always contains a
single, intact `</project_instructions>` terminator.

pkg/mcpclient/config.go-17-28 (1)

17-28: ⚠️ Potential issue | 🟡 Minor

Handle missing .mcp.json paths in NormalizeMCPDir().

The comment says a file path like /path/to/.mcp.json is accepted, but if that file does not exist os.Stat() fails and the path is returned unchanged. LoadMCPConfig() then probes /path/to/.mcp.json/.mcp.json and returns an error instead of (nil, nil).

🛠️ Proposed fix

 func NormalizeMCPDir(dir string) string {
+	if filepath.Base(dir) == MCPConfigFileName {
+		return filepath.Dir(dir)
+	}
 	info, err := os.Stat(dir)
 	if err == nil && !info.IsDir() {
 		return filepath.Dir(dir)
 	}
 	return dir
 }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@pkg/mcpclient/config.go` around lines 17 - 28, NormalizeMCPDir currently
returns the input unchanged when os.Stat fails, which causes LoadMCPConfig to
append MCPConfigFileName twice for inputs like "/path/to/.mcp.json" that don't
yet exist; update NormalizeMCPDir to detect if filepath.Base(dir) ==
MCPConfigFileName and in that case return filepath.Dir(dir) (even if os.Stat
returns an error), otherwise keep the existing behavior (if os.Stat succeeds and
info.IsDir() is false return filepath.Dir(dir), else return dir). Refer to the
functions NormalizeMCPDir and LoadMCPConfig and the constant MCPConfigFileName
when making this change.

emain/emain-web.ts-52-65 (1)

52-65: ⚠️ Potential issue | 🟡 Minor

Register the load waiter before calling reload().

At Lines 53-59, wc.reload() fires before the did-finish-load listener is attached. On a fast or cached reload that can miss the event, force the timeout path, and then run executeJavaScript() while navigation is still in flight.

🛠️ Proposed fix

-        wc.reload();
-        await new Promise<void>((resolve) => {
-            const onFinish = () => {
-                wc.removeListener("did-finish-load", onFinish);
-                resolve();
-            };
-            wc.on("did-finish-load", onFinish);
-            // Timeout fallback in case did-finish-load doesn't fire
-            setTimeout(() => {
-                wc.removeListener("did-finish-load", onFinish);
-                resolve();
-            }, 10000);
-        });
+        await new Promise<void>((resolve) => {
+            const onFinish = () => {
+                clearTimeout(timeoutId);
+                resolve();
+            };
+            const timeoutId = setTimeout(() => {
+                wc.removeListener("did-finish-load", onFinish);
+                resolve();
+            }, 10000);
+            wc.once("did-finish-load", onFinish);
+            wc.reload();
+        });

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@emain/emain-web.ts` around lines 52 - 65, The did-finish-load listener is
registered after calling wc.reload(), which can miss the event on fast loads;
move the listener registration for "did-finish-load" (the onFinish callback used
in the Promise) so it is attached to wc before invoking wc.reload(), keep the
10s timeout fallback and the wc.removeListener cleanup inside both the onFinish
and timeout branches, and then await that Promise before calling
executeJavaScript() so navigation has completed.

frontend/app/aipanel/aipanel.tsx-312-321 (1)

312-321: ⚠️ Potential issue | 🟡 Minor

Don't turn canceled requests into fake connection failures.

This branch rewrites any "aborted" error into a network banner, so a normal user cancel/stop path will look like connectivity broke.

🧯 Proposed fix

             let errorMsg = error.message || "An error occurred";
+            const normalized = errorMsg.toLowerCase();
+            if (normalized.includes("aborted")) {
+                return;
+            }
             if (
-                errorMsg.includes("fetch") ||
-                errorMsg.includes("network") ||
-                errorMsg.includes("ECONNREFUSED") ||
-                errorMsg.includes("Failed to fetch") ||
-                errorMsg.includes("timeout") ||
-                errorMsg.includes("aborted")
+                normalized.includes("fetch") ||
+                normalized.includes("network") ||
+                normalized.includes("econnrefused") ||
+                normalized.includes("failed to fetch") ||
+                normalized.includes("timeout")
             ) {
                 errorMsg = "Connection failed — check your internet connection and try again.";
             }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@frontend/app/aipanel/aipanel.tsx` around lines 312 - 321, The code is turning
cancelled requests into network-failure messages by treating any error message
containing "aborted" as a connection failure; update the logic around errorMsg
so that cancellations are not mapped to the network banner—remove "aborted" from
the generic includes check and instead detect real user/request cancellations
explicitly (e.g., check error.name === "AbortError" or error.code ===
"ERR_CANCELED" or instanceof DOMException with name "AbortError") and skip
mapping those to "Connection failed — check your internet connection and try
again." Reference the errorMsg variable and the includes-based condition to
locate and change the check.

pkg/aiusechat/aiplan/handler.go-30-37 (1)

30-37: ⚠️ Potential issue | 🟡 Minor

Use a non-safe verb for plan deletion.

This handler mutates state but accepts GET and currently doesn't reject any other method. That makes the delete route easier to trigger accidentally and inconsistent with the method guard in HandlePlanStatus. Please switch this endpoint to POST/DELETE and reject other methods here, with the caller in frontend/app/aipanel/planprogress.tsx updated to match.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@pkg/aiusechat/aiplan/handler.go` around lines 30 - 37, HandlePlanDelete
currently performs state mutation on an HTTP GET and does not guard methods;
change it to accept only POST or DELETE (pick one) and return a 405 for other
methods. Update the handler function HandlePlanDelete to check r.Method (e.g.,
allow "POST" or "DELETE" only), call DeletePlan(tabId) as before when allowed,
and write a proper JSON error response and 405 status for disallowed methods;
also update the caller in frontend/app/aipanel/planprogress.tsx to use the
chosen method instead of GET. Ensure Content-Type and success response remain
consistent when the delete is performed.

frontend/app/aipanel/sessionhistory.tsx-35-48 (1)

35-48: ⚠️ Potential issue | 🟡 Minor

Scope banner UI state to tabId.

session, expanded, and dismissed survive tab switches. A dismissed banner on one tab suppresses fetches for the next tab, and the previous tab's history can stay visible until the new request resolves. Reset or key these states by tabId before fetching the next session.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@frontend/app/aipanel/sessionhistory.tsx` around lines 35 - 48, The session,
expanded, and dismissed states currently persist across tab switches causing
stale UI and suppressed fetches; update the logic in the useEffect (and
surrounding component) to reset those states whenever tabId changes before
calling fetchSessionHistory: on tabId change call setSession(null),
setExpanded(false), setDismissed(false) (or derive these states keyed by tabId)
then invoke fetchSessionHistory(tabId) and setSession with the result; ensure
the cancellation guard (cancelled) still prevents races and keep tabId in the
dependency array so each tab gets a fresh fetch and UI state.

frontend/app/aipanel/planprogress.tsx-82-101 (1)

82-101: ⚠️ Potential issue | 🟡 Minor

Ignore stale fetches when tabId changes.

plan and minimized are carried across tab switches, and the polling effect doesn't guard against in-flight requests from the previous tab. That can briefly render the wrong plan or restore an old plan after a switch. Scope these states to tabId and ignore stale responses in the effect.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@frontend/app/aipanel/planprogress.tsx` around lines 82 - 101,
PlanProgressPanel currently preserves plan and minimized across tab switches and
lets in-flight fetchPlan calls from a previous tab overwrite state; scope these
states to tabId and ignore stale responses by resetting plan and minimized when
tabId changes and using a request token or AbortController inside the
useEffect/refresh flow to validate responses. Specifically, when tabId changes,
call setPlan(null) and setMinimized(false), and in refresh (used by
useEffect/setInterval) capture a local requestId or attach an AbortController
for each fetchPlan call and only call setPlan(result) if the requestId matches
the latest or the controller hasn't been aborted; also ensure the interval is
cleared and any controller aborted on cleanup so no stale promises update state.
Ensure dismissPlan continues to use the current tabId token/abort behavior so it
cannot revive state for a different tab.

frontend/app/aipanel/apikeyinput.tsx-17-34 (1)

17-34: ⚠️ Potential issue | 🟡 Minor

Guard handleSave against re-entry.

Enter still calls handleSave() while a save is in flight, so repeated key presses can enqueue multiple SetSecretsCommand requests even though the button is disabled. Add saving to the early-return guard and dependency list, and optionally disable the input while saving.

💡 Minimal fix

 const handleSave = useCallback(async () => {
-    if (!apiKey.trim() || !inputData) return;
+    if (saving || !apiKey.trim() || !inputData) return;
     setSaving(true);
     try {
         // Save API key as secret
         await RpcApi.SetSecretsCommand(TabRpcClient, {
             [inputData.secretName]: apiKey.trim(),
@@
-    }, [apiKey, inputData, model]);
+    }, [apiKey, inputData, model, saving]);

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@frontend/app/aipanel/apikeyinput.tsx` around lines 17 - 34, The handleSave
function can be re-entered while a save is in flight; update handleSave to
return early when saving is true (add "if (saving) return" to the existing
guard), include saving in the useCallback dependency array, and optionally
disable the input/UI while saving so users can’t trigger another save; locate
the handleSave function and related state (apiKey, inputData, saving, model) to
apply these changes.

pkg/aiusechat/tools_mcp.go-47-66 (1)

47-66: ⚠️ Potential issue | 🟡 Minor

Consider using the focused block from LayoutState instead of the first terminal.

getTerminalCwd returns the first terminal block with a cmd:cwd, but the Tab model exposes focused-block information via LayoutState.FocusedNodeId and LeafOrder. On tabs with multiple terminals, this can point MCP at a different project than the one actively in focus. Consider fetching the LayoutState (using tab.LayoutState) and resolving the focused node through LeafOrder to locate the active terminal before falling back to the first terminal.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@pkg/aiusechat/tools_mcp.go` around lines 47 - 66, getTerminalCwd currently
returns the first terminal block's cwd; change it to prefer the focused terminal
from the Tab's layout state: fetch tab.LayoutState, read
LayoutState.FocusedNodeId and resolve it through LayoutState.LeafOrder (or
equivalent mapping) to get the focused blockId, then try to load that block and
return its "cmd:cwd" if it's a terminal; if that lookup fails or the focused
node isn't a terminal, fall back to iterating tab.BlockIds as the current
implementation does. Update references in getTerminalCwd to use tab.LayoutState,
FocusedNodeId, and LeafOrder when resolving the active block before the existing
loop.

frontend/app/aipanel/mcpdetect.tsx-68-91 (1)

68-91: ⚠️ Potential issue | 🟡 Minor

Reset the dismissed state when the tab changes.

dismissed survives prop changes. After a user dismisses the banner in one tab, the early return on Lines 71-74 keeps suppressing MCP detection in later tabs too.

🪟 Simple reset

     const [detectInfo, setDetectInfo] = useState<MCPDetectInfo | null>(null);
     const [dismissed, setDismissed] = useState(false);
+
+    useEffect(() => {
+        setDismissed(false);
+    }, [tabId]);
 
     useEffect(() => {
         if (mcpEnabled || dismissed) {
             setDetectInfo(null);

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@frontend/app/aipanel/mcpdetect.tsx` around lines 68 - 91, The dismissed flag
is persisting across tabs and suppressing detection; add a short effect that
resets dismissed when the active tab changes by adding useEffect(() => {
setDismissed(false); }, [tabId]) (referencing dismissed and setDismissed) so
each new tab re-runs MCP detection instead of inheriting the previous dismissal.

pkg/mcpclient/handler.go-146-157 (1)

146-157: ⚠️ Potential issue | 🟡 Minor

Same non-deterministic map iteration for server selection.

Similar to HandleMCPStatus, when req.Server is empty, the code iterates over configs map non-deterministically to pick a default server.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@pkg/mcpclient/handler.go` around lines 146 - 157, The code picks a default
server by iterating the configs map non-deterministically (serverName loop)
which can vary between runs; change this to select deterministically by
collecting the keys from LoadMCPConfig(req.Cwd) into a slice, sort them (e.g.,
alphabetical), and then set serverName to the first sorted key (or prefer a
named default key if present) before proceeding; update the same logic used in
HandleMCPStatus to use the sorted-key selection so serverName is deterministic.

pkg/aiusechat/aiplan/aiplan.go-102-141 (1)

102-141: ⚠️ Potential issue | 🟡 Minor

UpdateStep does not validate status parameter against allowed values.

The function accepts any string as status without validating it's one of the defined constants (StatusPending, StatusRunning, StatusDone, StatusFailed, StatusSkipped). This could lead to inconsistent plan states.

🛡️ Proposed fix to validate status

 func UpdateStep(tabId string, stepId int, status string, result string, errMsg string) (*Plan, error) {
+	// Validate status is a known value
+	switch status {
+	case StatusPending, StatusRunning, StatusDone, StatusFailed, StatusSkipped:
+		// valid
+	default:
+		return nil, fmt.Errorf("invalid status: %s", status)
+	}
+	
 	plansMu.Lock()
 	defer plansMu.Unlock()

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@pkg/aiusechat/aiplan/aiplan.go` around lines 102 - 141, UpdateStep currently
accepts any string for the status parameter which can create invalid plan
states; add validation at the start of UpdateStep to ensure status is one of the
allowed constants (StatusPending, StatusRunning, StatusDone, StatusFailed,
StatusSkipped), return a descriptive error if not, and only apply the status
update to the matching step after validation; reference the UpdateStep function
and the status constants when implementing this check and ensure existing
behavior (result/error truncation, DoneAt timestamp, savePlanLocked) remains
unchanged.

pkg/mcpclient/handler.go-76-116 (1)

76-116: ⚠️ Potential issue | 🟡 Minor

Non-deterministic server selection when multiple servers are configured.

The loop for name := range configs iterates over a map, which has non-deterministic order in Go. This means the "first" server selected could vary between calls, leading to inconsistent behavior. If there are multiple servers and only one connects successfully, the user may get different results on repeated calls.

Consider sorting the server names for deterministic selection, or documenting this behavior:

♻️ Proposed fix for deterministic server selection

+import "sort"
+
 func HandleMCPStatus(w http.ResponseWriter, r *http.Request) {
 	...
+	// Sort server names for deterministic selection
+	var names []string
+	for name := range configs {
+		names = append(names, name)
+	}
+	sort.Strings(names)
+
 	var resp MCPStatusResponse
-	for name := range configs {
+	for _, name := range names {
 		client, err := manager.GetClient(cwd, name)

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@pkg/mcpclient/handler.go` around lines 76 - 116, Handle the non-deterministic
map iteration in HandleMCPStatus by collecting the keys from configs, sorting
them, and iterating over the sorted slice instead of using "for name := range
configs"; this ensures deterministic server selection when calling
manager.GetClient(cwd, name) and building resp (using client.IsAlive(),
client.ServerInfo(), client.ListTools(), client.ListResources()) so repeated
calls produce consistent results.

frontend/app/aipanel/aimessage.tsx (1)

130-135: Consider handling the case when no terminal block exists.

When findFirstTerminalBlockId() returns null, the function silently returns without feedback. Users clicking the execute button may not understand why nothing happened.

Additionally, RpcApi.ControllerInputCommand returns a Promise that isn't awaited or caught, which could lead to unhandled rejections if the RPC fails.

♻️ Suggested improvement with error handling

-function sendCommandToTerminal(cmd: string) {
+async function sendCommandToTerminal(cmd: string): Promise<boolean> {
     const blockId = findFirstTerminalBlockId();
-    if (!blockId) return;
+    if (!blockId) {
+        console.warn("No terminal block found to execute command");
+        return false;
+    }
     const b64data = stringToBase64(cmd + "\n");
-    RpcApi.ControllerInputCommand(TabRpcClient, { blockid: blockId, inputdata64: b64data });
+    try {
+        await RpcApi.ControllerInputCommand(TabRpcClient, { blockid: blockId, inputdata64: b64data });
+        return true;
+    } catch (e) {
+        console.error("Failed to send command to terminal", e);
+        return false;
+    }
 }

Alternatively, if visual feedback is desired, the callback could trigger a toast/notification when no terminal is available.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@frontend/app/aipanel/aimessage.tsx` around lines 130 - 135, The
sendCommandToTerminal function silently returns when findFirstTerminalBlockId()
yields null and does not handle RPC errors; update sendCommandToTerminal to (1)
provide user feedback when no terminal block exists (e.g., call an existing
toast/notification helper or emit an event) instead of silently returning, and
(2) await RpcApi.ControllerInputCommand(TabRpcClient, {...}) and wrap it in
try/catch to handle and log/display any error; reference the function
sendCommandToTerminal, findFirstTerminalBlockId, and
RpcApi.ControllerInputCommand when making the changes.

pkg/aiusechat/chatstore/chatstore.go (1)

52-62: GetAll() should deep-copy chats for consistency with Get().

GetAll() creates a new map but shares the underlying *uctypes.AIChat pointers with the store. This is inconsistent with Get(), which deep-copies the chat (including the NativeMessages slice) with an explicit comment stating it's to "prevent concurrent access issues." While current usage is read-only and safe, this shallow-copy pattern creates a maintenance risk—future callers might mutate returned chats and cause data races.

Align GetAll() with Get() by deep-copying each chat:

Proposed fix

 func (cs *ChatStore) GetAll() map[string]*uctypes.AIChat {
 	cs.lock.Lock()
 	defer cs.lock.Unlock()

 	result := make(map[string]*uctypes.AIChat, len(cs.chats))
 	for k, v := range cs.chats {
-		result[k] = v
+		copyChat := &uctypes.AIChat{
+			ChatId:         v.ChatId,
+			APIType:        v.APIType,
+			Model:          v.Model,
+			APIVersion:     v.APIVersion,
+			NativeMessages: make([]uctypes.GenAIMessage, len(v.NativeMessages)),
+		}
+		copy(copyChat.NativeMessages, v.NativeMessages)
+		result[k] = copyChat
 	}
 	return result
 }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@pkg/aiusechat/chatstore/chatstore.go` around lines 52 - 62, GetAll currently
returns a new map but reuses the store's *uctypes.AIChat pointers, risking
races; change GetAll to deep-copy each chat the same way Get() does (including
copying the NativeMessages slice and any nested fields) before inserting into
the result map. Locate GetAll and either call the existing Get() or
extract/reuse the deep-copy logic used by Get() to produce independent
*uctypes.AIChat instances while still holding cs.lock during the iteration, then
return the map of those copies.

frontend/app/aipanel/aimode.tsx (1)

152-157: Optimize to fetch secrets only when dropdown opens.

The effect runs on every isOpen change, including when closing the dropdown. This causes unnecessary API calls. Consider guarding the fetch to only run when isOpen is true.

♻️ Suggested fix

     // Load available secret names to filter byok presets
     useEffect(() => {
+        if (!isOpen) return;
         RpcApi.GetSecretsNamesCommand(TabRpcClient)
             .then((names) => setAvailableSecrets(new Set(names || [])))
             .catch(() => {});
     }, [isOpen]);

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@frontend/app/aipanel/aimode.tsx` around lines 152 - 157, The effect currently
triggers on every isOpen change (both open and close) causing unnecessary API
calls; update the useEffect that calls
RpcApi.GetSecretsNamesCommand(TabRpcClient) to first check that isOpen is true
and only then call the API, and ensure the result is passed to
setAvailableSecrets(new Set(names || [])); keep the existing .catch handler (or
add an abort/cleanup if you prefer) but do not call the API when isOpen is
false.

pkg/aiusechat/anthropic/anthropic-convertmessage.go (1)

147-150: LGTM on tool injection, but debug logging is incomplete.

The MCP tools are correctly cleaned and appended to the request, matching the pattern for TabTools. However, the debug logging below (lines 161-166) only collects tool names from chatOpts.Tools and chatOpts.TabTools, omitting chatOpts.MCPTools. This could make debugging MCP tool issues harder.

♻️ Suggested fix to include MCP tools in debug logging

 		for _, tool := range chatOpts.Tools {
 			toolNames = append(toolNames, tool.Name)
 		}
 		for _, tool := range chatOpts.TabTools {
 			toolNames = append(toolNames, tool.Name)
 		}
+		for _, tool := range chatOpts.MCPTools {
+			toolNames = append(toolNames, tool.Name)
+		}
 		if chatOpts.AllowNativeWebSearch {
 			toolNames = append(toolNames, "web_search[server]")
 		}

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@pkg/aiusechat/anthropic/anthropic-convertmessage.go` around lines 147 - 150,
The debug logging that builds the tool-name list currently iterates
chatOpts.Tools and chatOpts.TabTools but omits chatOpts.MCPTools, so update the
logging code to also iterate chatOpts.MCPTools (using the same Clean() or name
extraction logic used for other tool slices) and include those names in the
debug output; look for the block that collects tool names for logging and add a
loop over chatOpts.MCPTools (or map/append its cleaned names) so MCP tools
appear in the debug message alongside TabTools and Tools.

pkg/web/web.go (1)

24-27: Import ordering is slightly out of alphabetical order.

The imports are functional but mcpclient (line 27) should come after filestore alphabetically. This is a minor style issue.

♻️ Suggested import reordering

 	"github.com/wavetermdev/waveterm/pkg/aiusechat"
 	"github.com/wavetermdev/waveterm/pkg/aiusechat/aiplan"
 	"github.com/wavetermdev/waveterm/pkg/aiusechat/sessionhistory"
 	"github.com/wavetermdev/waveterm/pkg/authkey"
-	"github.com/wavetermdev/waveterm/pkg/mcpclient"
 	"github.com/wavetermdev/waveterm/pkg/filestore"
+	"github.com/wavetermdev/waveterm/pkg/mcpclient"
 	"github.com/wavetermdev/waveterm/pkg/panichandler"

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@pkg/web/web.go` around lines 24 - 27, The import block in pkg/web/web.go is
out of alphabetical order: move the
"github.com/wavetermdev/waveterm/pkg/mcpclient" import so it appears after
"github.com/wavetermdev/waveterm/pkg/filestore" (i.e., sort the imports
alphabetically) to match project style; update the import ordering around the
existing imports like aiplan, sessionhistory, authkey, filestore and mcpclient
so mcpclient comes in the correct position.

pkg/aiusechat/sessionhistory/handler.go (1)

25-34: Reuse the package loader instead of duplicating file reads here.

This handler re-implements path lookup, ReadFile, and JSON unmarshal rather than going through LoadSessionHistory. Keeping two storage paths makes format/path changes easy to drift, and unexpected read/parse failures are currently flattened to {"entries": null} with no shared handling.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@pkg/aiusechat/sessionhistory/handler.go` around lines 25 - 34, The handler
currently duplicates file reading and JSON unmarshal (using getHistoryFilePath,
os.ReadFile, and json.Unmarshal into SessionLog) instead of the shared loader;
replace that block with a call to the package loader LoadSessionHistory(tabId)
and use its returned SessionLog (or error) so path/format logic is centralized;
on error, call the same shared response handling (writeEmptyResponse or the
loader's error handling) and remove the manual os.ReadFile/json.Unmarshal code
to avoid divergence.

pkg/aiusechat/usechat.go (4)

768-775: MCP CWD fallback duplicates CWD lookup logic.

This block fetches getTerminalCwd again even though it may have been called earlier (line 748). If the consolidation above is applied, tabCwd can be reused here.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@pkg/aiusechat/usechat.go` around lines 768 - 775, The MCP CWD fallback calls
getTerminalCwd again instead of reusing the previously computed tabCwd; update
the block that sets chatOpts.MCPCwd (guarded by req.MCPAccess and checking
req.MCPCwd) to assign chatOpts.MCPCwd = req.MCPCwd when present and otherwise
set chatOpts.MCPCwd = tabCwd (the variable computed earlier) rather than calling
getTerminalCwd again; reference req.MCPAccess, req.MCPCwd, chatOpts.MCPCwd and
tabCwd when making the change.

---

193-211: Timeout error wrapping may mask the original error.

When the context is done, the error is replaced with a generic timeout message. This loses the original error information which could be valuable for debugging (e.g., distinguishing between network errors vs. actual timeouts).

Consider preserving the original error:

 	if err != nil {
 		if ctx.Err() != nil {
-			return nil, nil, fmt.Errorf("request timed out - check your internet connection and try again")
+			return nil, nil, fmt.Errorf("request timed out - check your internet connection and try again: %w", err)
 		}
 	}

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@pkg/aiusechat/usechat.go` around lines 193 - 211, In runAIChatStep, avoid
discarding the original error returned by backend.RunChatStep when ctx.Err() !=
nil; instead wrap or append the original err (from the RunChatStep call) into
the timeout message so the caller can inspect the underlying cause (e.g., use
fmt.Errorf("request timed out: %w", err) or include err.Error()), and only
replace the error when there is truly no underlying err; reference the
RunChatStep call and the ctx.Err() check to locate where to change the error
handling.

---

731-761: Multiple redundant tab ID checks and terminal CWD lookups.

The code checks req.TabId != "" four separate times (lines 733, 738, 747, 756) and calls getTerminalCwd in two places (749, 773). Consider consolidating these checks and caching the CWD lookup result:

♻️ Suggested consolidation

+	var tabCwd string
 	if req.TabId != "" {
 		sessionhistory.RegisterChatTab(req.ChatID, req.TabId)
-	}
-
-	// Notify AI about previous session history availability
-	if req.TabId != "" {
+		tabCwd = getTerminalCwd(r.Context(), req.TabId)
+		
+		// Notify AI about previous session history availability
 		prevSession := sessionhistory.LoadSessionHistory(req.TabId)
 		if prevSession != "" {
 			chatOpts.SystemPrompt = append(chatOpts.SystemPrompt,
 				"A previous session history exists...")
 		}
-	}
 
-	// Notify AI about project instructions file availability
-	if req.TabId != "" {
-		cwd := getTerminalCwd(r.Context(), req.TabId)
-		if cwd != "" && projectctx.FindInstructionsFile(cwd) != "" {
+		// Notify AI about project instructions file availability
+		if tabCwd != "" && projectctx.FindInstructionsFile(tabCwd) != "" {
 			chatOpts.SystemPrompt = append(chatOpts.SystemPrompt,
 				"Project coding instructions file found...")
 		}
-	}
 
-	// Inject active plan status if one exists for this tab
-	if req.TabId != "" {
+		// Inject active plan status if one exists for this tab
 		activePlan := aiplan.GetPlan(req.TabId)
 		if activePlan != nil && !aiplan.IsComplete(activePlan) {
 			chatOpts.SystemPrompt = append(chatOpts.SystemPrompt, aiplan.FormatPlanStatus(activePlan))
 		}
 	}

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@pkg/aiusechat/usechat.go` around lines 731 - 761, Consolidate the repeated
req.TabId != "" guards and duplicate getTerminalCwd calls by performing a single
if req.TabId != "" block that (1) calls
sessionhistory.RegisterChatTab(req.ChatID, req.TabId), (2) loads prevSession via
sessionhistory.LoadSessionHistory(req.TabId) and appends the session_history
system prompt if non-empty, (3) call getTerminalCwd(r.Context(), req.TabId) once
and cache it to use with projectctx.FindInstructionsFile(cwd) to append the
project_instructions prompt if found, and (4) fetch activePlan via
aiplan.GetPlan(req.TabId) and append aiplan.FormatPlanStatus(activePlan) when
activePlan != nil && !aiplan.IsComplete(activePlan); reuse the cached cwd and
ensure all appends go to chatOpts.SystemPrompt.

---

457-465: MCP generation errors are silently logged without notifying the user.

When generateMCPStateAndTools fails, the error is logged but the chat continues without MCP context. This could lead to confusing behavior where the user expects MCP tools to be available but they aren't. Consider sending a warning to the SSE stream so the frontend can display it.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@pkg/aiusechat/usechat.go` around lines 457 - 465, When
generateMCPStateAndTools fails in the firstStep branch (the mcpErr != nil case),
emit a warning event to the SSE/chat stream so the frontend can display that MCP
tools/context are unavailable instead of silently continuing; keep setting
chatOpts.MCPState and chatOpts.MCPTools nil (or leave unchanged) and send a
short SSE payload (e.g., event type "warning" with a message including
mcpErr.Error()) so the UI can surface the problem. Locate the block using
generateMCPStateAndTools, mcpErr, chatOpts.MCPState and chatOpts.MCPTools and
call the project’s existing SSE emit function (or add one) to send a warning
event containing the error string and a clear user-facing message.

pkg/mcpclient/manager.go (2)

113-141: GetAutoContext does not limit the size of returned context.

If application-info or database-schema tools return very large outputs, the context string could become excessively large and consume significant tokens in the AI prompt. Consider adding truncation similar to truncateResult used elsewhere.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@pkg/mcpclient/manager.go` around lines 113 - 141, GetAutoContext builds an
unbounded context by appending raw outputs from client.CallTool for tools
"application-info" and "database-schema"; modify the code paths in
GetAutoContext (the loop over clients, the hasToolNamed + client.CallTool
blocks) to truncate long results before appending by calling the existing
truncateResult helper (or equivalent truncation util) with an appropriate max
length (same limit used elsewhere), and append the truncated string (and an
indicator like "...(truncated)") instead of the full result so the returned
context size is bounded.

---

221-231: Deleting from map while iterating is safe in Go, but could be clearer.

While Go allows deleting from a map during iteration, it's a pattern that can confuse readers. Consider collecting keys first or just iterating without deletion since clearing the map afterward would achieve the same result:

♻️ Alternative implementation

 func (m *MCPManager) Shutdown() {
 	m.mu.Lock()
 	defer m.mu.Unlock()
 
 	for key, client := range m.clients {
 		client.Close()
-		delete(m.clients, key)
+		_ = key // suppress unused warning
 	}
+	m.clients = make(map[string]*MCPClient)
 	log.Println("[mcpclient] all MCP clients shut down")
 }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@pkg/mcpclient/manager.go` around lines 221 - 231, The Shutdown method of
MCPManager currently deletes map entries while ranging (MCPManager.Shutdown
iterating m.clients and calling client.Close() then delete), which is legal but
confusing; change it to lock with m.mu.Lock(), iterate over m.clients and call
client.Close() for each entry without deleting, then clear the map in one step
(e.g., set m.clients = make(map[string]*MCPClient) or nil) after the loop to
make intent clear; keep the defer m.mu.Unlock() and ensure you still call
client.Close() on each MCP client.

pkg/aiusechat/aiplan/aiplan.go (1)

143-148: DeletePlan silently ignores removal errors.

The os.Remove error is not checked. While this may be intentional (e.g., file doesn't exist), unexpected errors like permission issues would go unnoticed. Consider at least logging the error.

♻️ Proposed fix to log errors

 func DeletePlan(tabId string) {
 	plansMu.Lock()
 	defer plansMu.Unlock()
-	os.Remove(getPlanFilePath(tabId))
+	if err := os.Remove(getPlanFilePath(tabId)); err != nil && !os.IsNotExist(err) {
+		log.Printf("[aiplan] warning: failed to delete plan for tab %s: %v", tabId, err)
+	}
 }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@pkg/aiusechat/aiplan/aiplan.go` around lines 143 - 148, The DeletePlan
function currently calls os.Remove(getPlanFilePath(tabId)) and ignores its
error; change it to capture the error (err := os.Remove(...)) and handle it: if
err != nil && !os.IsNotExist(err) then log the error (using the package logger
or the standard log package) so permission or other unexpected failures are
visible; keep the existing plansMu.Lock()/defer unlock and do not treat "file
not found" as an error.

pkg/mcpclient/handler.go (2)

208-211: writeJSON ignores encoding errors.

If json.NewEncoder(w).Encode(data) fails, the error is silently ignored. While rare, this could mask issues with response serialization.

♻️ Proposed fix to log errors

 func writeJSON(w http.ResponseWriter, data any) {
 	w.Header().Set("Content-Type", "application/json")
-	json.NewEncoder(w).Encode(data)
+	if err := json.NewEncoder(w).Encode(data); err != nil {
+		log.Printf("[mcpclient] warning: failed to encode JSON response: %v", err)
+	}
 }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@pkg/mcpclient/handler.go` around lines 208 - 211, The writeJSON function
currently ignores errors from json.NewEncoder(w).Encode(data); update writeJSON
to check the error returned by Encode and handle it (e.g., log the encoding
error and respond with an HTTP 500 if the response hasn't been written). Locate
writeJSON and change the call to capture err := json.NewEncoder(w).Encode(data),
then if err != nil use a logger or log.Printf to record the error (including the
data or context) and call http.Error(w, "internal server error",
http.StatusInternalServerError) or similar to notify the client.

---

125-130: Request body is read but defer r.Body.Close() comes after the read.

While technically not a bug (the body will be closed when the handler returns), it's more idiomatic to defer the close immediately after checking the error from ReadAll. More importantly, if ReadAll fails, the body should still be closed.

♻️ Suggested reorder

-	body, err := io.ReadAll(r.Body)
-	if err != nil {
-		writeJSON(w, MCPCallResponse{Error: "failed to read request body"})
-		return
-	}
-	defer r.Body.Close()
+	defer r.Body.Close()
+	body, err := io.ReadAll(r.Body)
+	if err != nil {
+		writeJSON(w, MCPCallResponse{Error: "failed to read request body"})
+		return
+	}

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@pkg/mcpclient/handler.go` around lines 125 - 130, Move the defer
r.Body.Close() so the request body is always closed even if io.ReadAll returns
an error: after calling io.ReadAll(r.Body) place defer r.Body.Close() before the
subsequent if err != nil check (or immediately upon entering the handler), then
keep the existing error branch that calls writeJSON(w, MCPCallResponse{Error:
...}) and returns; update only the ordering around io.ReadAll and r.Body.Close()
(references: io.ReadAll, r.Body.Close, MCPCallResponse, writeJSON).